-
Notifications
You must be signed in to change notification settings - Fork 670
chore: map maintenance err exit code #6418
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
a3f7df3 to
6bd606d
Compare
95f8ab1 to
20e2f5f
Compare
650d84f to
9118aee
Compare
9118aee to
dc74c57
Compare
| ); | ||
|
|
||
| describe('Special error cases', () => { | ||
| describe('network retries', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: Should this test that focusses on the network retry logic be in error-catalog.spec.ts? Seems more reasonable to move it to its own file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would the existing https.spec.ts be more suitable? If not, I'll create a new file - network.spec.ts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https.spec.ts would be okay for now. maybe we want to combine all network related tests at some point?!
| }, | ||
| ); | ||
|
|
||
| describe('maintenance error [SNYK-0099]', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: This test looks very much alike like this, why don't we just merge them. We basically just need to add the additional assertion on the request counts to the other test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the setup is quite similar. The separation is due to the test, testing network retries based on error catalog errors doesn't make sense in the exitcode.spec.ts file.
Alternatively, this test can be moved to network.spec.ts if needed
dc74c57 to
874ba61
Compare
9226cb2 to
4e275ec
Compare
4db41de to
56bd8a4
Compare
CLI-1268
56bd8a4 to
522cf42
Compare
Pull Request Submission Checklist
are release-note ready, emphasizing
what was changed, not how.
What does this PR do?
Adds exit code mapping for the MaintenanceError Error Catalog error.
Where should the reviewer start?
How should this be manually tested?
What's the product update that needs to be communicated to CLI users?